feat(pnv): Add support for Firebase Phone Number Verification#1203
feat(pnv): Add support for Firebase Phone Number Verification#1203lahirumaramba wants to merge 28 commits intomainfrom
Conversation
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the Firebase Phone Number Verification service, implementing JWT token verification using the Nimbus JOSE + JWT library. It includes the core service entry point, model classes for verified tokens, custom exception handling, and a verifier that checks JWT headers and claims against a remote JWKS. Feedback highlights a security vulnerability where the issuer claim was not validated against the specific project ID, potentially allowing tokens from other projects. Additionally, improvements were suggested for the token model to handle claim types more robustly and prevent potential class cast exceptions.
…abilize headless pipelines
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces the Firebase Phone Number Verification service to the Java Admin SDK. The changes include the main service entry point, internal token verification logic using the Nimbus JOSE + JWT library, custom exception handling, and a comprehensive suite of unit tests. Additionally, a new AGENTS.md file defines coding guidelines for AI assistants, and a minor formatting fix was applied to Template.java. Review feedback suggests improving null safety in the token verification logic by using a more robust equality check for the JWS algorithm.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces the Firebase Phone Number Verification service to the Java SDK, including the main service entry point, custom exception handling, and JWT token verification logic using the Nimbus library. It also adds a new AGENTS.md file to define repository-wide coding standards for AI assistants. The review feedback focuses on improving the robustness and consistency of the FirebasePhoneNumberVerificationTokenVerifier class, specifically by addressing the risks of calling overridable methods in constructors, adopting more idiomatic null-check patterns, and utilizing more specific exception types for internal configuration errors.
… URI.create().toURL()
…on for jwtProcessor to avoid constructor leak
…vate constructor overload
…ow IllegalStateException for hardcoded configuration failures
…ions checks with idiomatic checkNotNull patterns
…r null claims test case
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces the Firebase Phone Number Verification service to the Java SDK, including the main service entry point, token representation, and internal JWT verification logic using the Nimbus library. It also adds a new guidelines file for AI coding assistants and updates the project dependencies. A thread-safety issue was identified in the token verifier class, where a field used in a double-checked locking pattern lacks the volatile keyword, potentially leading to race conditions.
…r strict double-checked locking memory safety
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces the Firebase Phone Number Verification service, providing JWT verification logic, exception handling, and comprehensive unit tests, alongside new AI coding assistant guidelines. Feedback suggests aligning the audience claim validation with other Firebase services, improving type safety in the token model by using JWTClaimsSet directly, and adding explicit checks for issued-at and expiration claims to strengthen security.
I am having trouble creating individual review comments. Click here to see my feedback.
src/main/java/com/google/firebase/phonenumberverification/internal/FirebasePhoneNumberVerificationTokenVerifier.java (178)
The audience check currently requires the aud claim to contain the full issuer URL. In most Firebase services, the audience is expected to be the project ID. Please verify if this is the intended behavior for the Phone Number Verification service, as it might cause verification failures if the token only contains the project ID in its audience claim.
src/main/java/com/google/firebase/phonenumberverification/FirebasePhoneNumberVerificationToken.java (62-72)
The getAudience method manually parses the aud claim from a raw map. Since the token has already been processed by DefaultJWTProcessor, you could consider storing the JWTClaimsSet object directly in this class and using its typed getAudience() method for better type safety and consistency.
src/main/java/com/google/firebase/phonenumberverification/internal/FirebasePhoneNumberVerificationTokenVerifier.java (161-192)
It is recommended to explicitly verify the presence of iat (issued-at) and exp (expiration) claims in the verifyClaims method. While the underlying jwtProcessor checks expiration if present, ensuring these claims exist adds an extra layer of security for token validation.
4 participants
Add support for Firebase Phone Number Verification